Skip to content

Remove useless assumes from slice::iter(_mut) #111447

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
May 13, 2023
Merged

Remove useless assumes from slice::iter(_mut) #111447

merged 1 commit into from
May 13, 2023

Conversation

scottmcm
Copy link
Member

You were right in #111395 (comment),
r? @the8472

LLVM already removes these assumes while optimizing, as can be seen in https://rust.godbolt.org/z/KTfWKbdEM.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 10, 2023
@rustbot

This comment was marked as resolved.

# to view
@thomcc
Copy link
Member

thomcc commented May 10, 2023 •
edited
Loading

Looks great, r=me after green CI (sorry for stealing it, linked to me in Discord and I'm already here). I don't think a perf run is needed but lets not roll it up just in case.

@bors rollup=never
r? @thomcc

@rustbot rustbot assigned thomcc and unassigned the8472 May 10, 2023
@thomcc
Copy link
Member

thomcc commented May 10, 2023 •
edited
Loading

Oh wait, it's rollup=never so there's no reason not to r+ it now.

@bors r+

@bors
Copy link
Collaborator

bors commented May 10, 2023

📌 Commit 4cde41a80030c9ecf354e113756c6a816e4ad085 has been approved by thomcc

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 10, 2023

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 10, 2023
@bors
Copy link
Collaborator

bors commented May 11, 2023

⌛ Testing commit 4cde41a80030c9ecf354e113756c6a816e4ad085 with merge 9310adfaa815d4c86044af0e176e7c83788990e7...

@bors
Copy link
Collaborator

bors commented May 11, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 11, 2023
@rust-log-analyzer

This comment has been minimized.

# to view
@scottmcm
Copy link
Member Author

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2023
@scottmcm scottmcm force-pushed the remove-more-assumes branch from 4cde41a to e5e7eb7 Compare May 12, 2023 02:57
@scottmcm
Copy link
Member Author

Rebased atop #111395.

@bors r=thomcc

@bors
Copy link
Collaborator

bors commented May 12, 2023

📌 Commit e5e7eb764def1d3fd92eefd0ec63eda4970f8631 has been approved by thomcc

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 12, 2023

⌛ Testing commit e5e7eb764def1d3fd92eefd0ec63eda4970f8631 with merge ae19200d5e51f1c0a62d525d1adf56b63d5bc0ec...

@rust-log-analyzer

This comment has been minimized.

# to view
@bors
Copy link
Collaborator

bors commented May 12, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 12, 2023
@scottmcm scottmcm force-pushed the remove-more-assumes branch from e5e7eb7 to c50a2e1 Compare May 13, 2023 00:35
@scottmcm
Copy link
Member Author

Oh, interesting, this with something else that went in after I last rebased means that slice::Iter::new is now small enough that it gets mir-inlined. Rebased again and reblessed.

@bors r=thomcc

@bors
Copy link
Collaborator

bors commented May 13, 2023

📌 Commit c50a2e1 has been approved by thomcc

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 13, 2023
@bors
Copy link
Collaborator

bors commented May 13, 2023

⌛ Testing commit c50a2e1 with merge 16d3e18...

@bors
Copy link
Collaborator

bors commented May 13, 2023

☀️ Test successful - checks-actions
Approved by: thomcc
Pushing 16d3e18 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 13, 2023
@bors bors merged commit 16d3e18 into rust-lang:master May 13, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 13, 2023
This was referenced May 13, 2023
Merged
Closed
@scottmcm scottmcm deleted the remove-more-assumes branch May 13, 2023 07:03
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (16d3e18): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 1.1% [1.1%, 1.1%] 2
Improvements ✅
(primary)		 -0.8% [-1.7%, -0.3%] 85
Improvements ✅
(secondary)		 -0.7% [-1.4%, -0.4%] 46
All ❌✅ (primary) -0.8% [-1.7%, -0.3%] 85

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 2.8% [2.7%, 2.9%] 2
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -3.6% [-5.1%, -2.3%] 4
Improvements ✅
(secondary)		 -3.1% [-3.3%, -2.9%] 2
All ❌✅ (primary) -1.5% [-5.1%, 2.9%] 6

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -1.3% [-1.9%, -0.8%] 3
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -1.3% [-1.9%, -0.8%] 3

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.4% [0.0%, 5.7%] 56
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.2% [-1.4%, -0.0%] 11
Improvements ✅
(secondary)		 -0.1% [-0.1%, -0.1%] 4
All ❌✅ (primary) 0.3% [-1.4%, 5.7%] 67

Bootstrap: 659.629s -> 659.427s (-0.03%)

@rustbot rustbot added the perf-regression Performance regression. label May 13, 2023
@nnethercote
Copy link
Contributor

Perf improvements here massively outweigh the small number of regressions.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label May 14, 2023
@scottmcm scottmcm mentioned this pull request Aug 3, 2023
Merged
@scottmcm scottmcm mentioned this pull request Nov 24, 2024
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@thomcc thomcc

Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.
8 participants
@scottmcm @rustbot @thomcc @bors @rust-log-analyzer @rust-timer @nnethercote @the8472